Skip to content

Conversation

@JanProvaznik
Copy link
Member

@JanProvaznik JanProvaznik commented Dec 10, 2025

fixes #12484
This PR migrates several file I/O tasks to use the new multithreadable task API by implementing the \IMultiThreadableTask\ interface and using \TaskEnvironment\ for thread-safe path resolution. The aim is to be maximally compatible with prior behavior in terms of where it would throw

Changes

The following tasks now implement \IMultiThreadableTask\ and use \TaskEnvironment.GetAbsolutePath()\ instead of \Path.GetFullPath():

  • Copy.cs
  • Delete.cs
  • MakeDir.cs
  • RemoveDir.cs
  • Touch.cs

Why

\Path.GetFullPath()\ uses process-global state (current working directory) which is not thread-safe when multiple tasks run in parallel. The \TaskEnvironment.GetAbsolutePath()\ method provides thread-safe path resolution by using the task's execution context.

This enables these tasks to be safely executed in parallel when MSBuild's multithreading is enabled.

Copilot AI and others added 6 commits December 3, 2025 16:40
- Updated FileState.cs to accept TaskEnvironment parameter
- Migrated 7 File I/O tasks to IMultiThreadableTask:
  * Copy - added attribute, interface, TaskEnvironment property and updated path/environment access
  * Delete - migrated with proper path resolution
  * MakeDir - migrated with path deduplication
  * RemoveDir - migrated with directory operations
  * Touch - migrated with file timestamp operations
  * ReadLinesFromFile - migrated for file reading
  * WriteLinesToFile - migrated for file writing
- All tasks use MSBuildMultiThreadableTaskAttribute for routing
- All tasks implement IMultiThreadableTask for TaskEnvironment access
- Tasks use TaskEnvironment.GetAbsolutePath() for thread-safe path resolution
- Tasks use TaskEnvironment.GetEnvironmentVariable() for thread-safe environment access

Co-authored-by: JanProvaznik <[email protected]>
Copilot AI review requested due to automatic review settings December 10, 2025 16:21
@JanProvaznik JanProvaznik changed the title Remove null-conditional operators for TaskEnvironment in file I/O tasks Migrate file I/O tasks to multithreadable task API Dec 10, 2025
@JanProvaznik JanProvaznik requested a review from AR-May December 10, 2025 16:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR converts several file I/O tasks to support multi-threaded execution by implementing IMultiThreadableTask and using TaskEnvironment for thread-safe path resolution. The changes replace static path resolution methods with instance-based TaskEnvironment.GetAbsolutePath() calls to ensure proper path handling in concurrent scenarios.

Key Changes:

  • Added IMultiThreadableTask interface and [MSBuildMultiThreadableTask] attribute to 7 file I/O task classes
  • Introduced TaskEnvironment property to all modified tasks for thread-safe operations
  • Modified FileState constructor to require TaskEnvironment parameter and use it for path resolution

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/Tasks/Touch.cs Added multithreading support; uses TaskEnvironment for path resolution in TouchFile method
src/Tasks/RemoveDir.cs Added multithreading support; resolves paths using TaskEnvironment in Execute and RemoveDirectory methods
src/Tasks/MakeDir.cs Added multithreading support; uses TaskEnvironment for path resolution and deduplication
src/Tasks/FileState.cs Modified constructor to require TaskEnvironment parameter; uses it for absolute path resolution in Lazy initialization
src/Tasks/FileIO/WriteLinesToFile.cs Added multithreading support; uses TaskEnvironment for path resolution instead of FileUtilities.NormalizePath
src/Tasks/FileIO/ReadLinesFromFile.cs Added multithreading support; resolves file paths using TaskEnvironment before file operations
src/Tasks/Delete.cs Added multithreading support; uses TaskEnvironment for path resolution in delete operations
src/Tasks/Copy.cs Added multithreading support; uses TaskEnvironment for path resolution, environment variables, and FileState construction; changed PathsAreIdentical from static to instance method

@AR-May AR-May self-assigned this Dec 12, 2025
Copy link
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I would like one more review to ensure there is no behavior change in multi process mode.

AR-May added a commit that referenced this pull request Jan 21, 2026
### Context
PRs #12914 and #12868 showed that we need to be able to fix and
normalize `AbsolutePath`. Corresponding methods for strings live in
`FileUtilities.cs` that is a Shared file and cannot reference Framework
classes like `AbsolutePath`.

### Changes Made
- Moved some related functions to FrameworkFileUtilities class.
- Added the needed functions for `AbsolutePath`

### Testing
unit tests
johnazule pushed a commit to johnazule/msbuild that referenced this pull request Jan 22, 2026
…#13079)

### Context
PRs dotnet#12914 and dotnet#12868 showed that we need to be able to fix and
normalize `AbsolutePath`. Corresponding methods for strings live in
`FileUtilities.cs` that is a Shared file and cannot reference Framework
classes like `AbsolutePath`.

### Changes Made
- Moved some related functions to FrameworkFileUtilities class.
- Added the needed functions for `AbsolutePath`

### Testing
unit tests
@JanProvaznik JanProvaznik merged commit eeb15d6 into dotnet:main Jan 26, 2026
10 checks passed
@YuliiaKovalova
Copy link
Member

YuliiaKovalova commented Jan 26, 2026 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate Copy task to the new task API

5 participants